Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add errors for Matrix ± AbstractJuMPScalar #3557

Merged
merged 7 commits into from
Nov 2, 2023
Merged

Add errors for Matrix ± AbstractJuMPScalar #3557

merged 7 commits into from
Nov 2, 2023

Conversation

jd-foster
Copy link
Collaborator

@jd-foster jd-foster commented Nov 2, 2023

Give a more informative error when there an operation between a single JuMP variable and a Matrix type object is attempted.

Closes #3553

src/operators.jl Outdated Show resolved Hide resolved
src/operators.jl Outdated Show resolved Hide resolved
@blegat
Copy link
Member

blegat commented Nov 2, 2023

Looks good to me, just need tests

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b11f744) 98.19% compared to head (ff19d49) 98.19%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3557   +/-   ##
=======================================
  Coverage   98.19%   98.19%           
=======================================
  Files          37       37           
  Lines        5602     5608    +6     
=======================================
+ Hits         5501     5507    +6     
  Misses        101      101           
Files Coverage Δ
src/operators.jl 96.03% <100.00%> (+0.10%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@odow odow marked this pull request as ready for review November 2, 2023 17:56
src/operators.jl Outdated Show resolved Hide resolved
src/operators.jl Outdated Show resolved Hide resolved
src/operators.jl Outdated Show resolved Hide resolved
src/operators.jl Outdated Show resolved Hide resolved
jd-foster and others added 2 commits November 3, 2023 07:19
Co-authored-by: Oscar Dowson <odow@users.noreply.github.com>
Co-authored-by: Oscar Dowson <odow@users.noreply.github.com>
@odow
Copy link
Member

odow commented Nov 2, 2023

Let's just double check with https://github.com/jump-dev/JuMP.jl/actions/runs/6738331476

(cc @mortenpi, here's an example where this is useful)

@odow
Copy link
Member

odow commented Nov 2, 2023

We can do separately, but with a slight tweak, this could also fix #3554

They're essentially the same problem.

@jd-foster
Copy link
Collaborator Author

If this OK to merge? What's the thing about actions and Morten?

@odow
Copy link
Member

odow commented Nov 2, 2023

LGTM.

The Morten thing is JuliaDocs/Documenter.jl#2323

@odow odow merged commit 944e5c5 into master Nov 2, 2023
22 of 23 checks passed
@odow odow deleted the jdf/mat+-scalar branch November 2, 2023 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Multiplying JuMP expression with UniformScaling absorbs the matrix
3 participants